Track downloaded streams and surface status on video detail by jmandel · Pull Request #12639 · TeamNewPipe/NewPipe

jmandel · 2025-09-17T02:34:15Z

What is it?

Bugfix (user facing)
Feature (user facing)
Codebase improvement (dev facing)
Meta improvement to the project (dev facing)

Description of the changes in your PR

add a downloaded_streams Room table/DAO, migration v10, and generated schema JSON so NewPipe can
remember which videos were downloaded (with SAF URIs, quality label, etc.)
hook the downloader enqueue/finish paths to insert/update those rows, and add lightweight self-
healing (video-open probe, downloads-screen refresh, periodic WorkManager job)
show a “Downloaded …” chip on the video detail page; when tapped it opens a themed sheet with:
- Open file, Delete file (also unlinks), Remove link, Show in folder when the file is still
  present
- Remove link when the file is gone
deleting or removing the link immediately hides the chip

Before/After Screenshots/Screen Record

Before: no indication that the stream was already downloaded; tapping “Download” always enqueued a
new mission.
After: a “Downloaded • 720p” chip appears above the metadata; tapping it reveals the action sheet
described above. (No visual regression screenshots attached.)

screen-20250916-223018.2.mp4

Fixes the following issue(s)

Fixes When trying to see a video, warn the user if the video has already been downloaded #7360

Relies on the following changes

(none)

Stypox · 2025-09-17T08:53:09Z

Thank you for this cool PR! I have some general comments:

Is there a reason why you added new database tables to keep track of downloads, when the downloader already has its own tables in the database? See

NewPipe/app/src/main/java/us/shandian/giga/get/sqlite/FinishedMissionStore.java

Line 25 in 1164ea5

public class FinishedMissionStore extends SQLiteOpenHelper {

. This class is veeeery old and is for sure bad code, so you are free to create new DAOs, but please don't create new redundant tables ;-)
New UI code on the refactor branch should ideally be written in Jetpack Compose instead of XML
Could you move out the code related to this button from the VideoDetailFragment file, so it doesn't get too complicated?

jmandel · 2025-09-17T12:25:33Z

Hey thank you for the encouraging feedback! I figured it was good to get a rough demo in shape before spending time on anything else.

(This pass was planned by GPT-5 Pro with limited access to the source code which is probably why I overlooked the existing table ;) I'm setting the PR to draft and will bring it back for review when I have addressed your feedback.)

Stypox · 2025-09-17T15:19:25Z

Oh, I see. If you use LLMs to make contributions to open source projects, I strongly suggest making sure you only use LLMs as a companion to get some help, not to blindly generate code, as the latter just creates unneeded review work for maintainers (which would be able to use LLMs themselves if they really wanted to).

Restore finshed mission conditoina

jmandel · 2025-09-17T16:43:12Z

Definitely doing the best I can to get up to speed on a new codebase. I haven't done much android development in the kotlin era :)

(Meta-level perspective: deciding what problems are worth tackling, coming up with a design, implementing it, and testing all still require significant nvestment, even with help from LLMs, so I'm not sure "maintainers would just use LLMs to do it all themselves if they wanted to" entirely holds -- e.g., #7360 has been open for almost 4 years, and I think I've made decent progress with 3-4 hours of my own time investment, plus your generous feedback + advice here. For me, part of the joy of open source is letting users learn, propose, and contribute things they care about.)

That said: I don't want to waste your time if you're not interested in this contribution -- it's your project, your rules, @Stypox!

I've updated this PR to address your initial feedback, but please let me know if you'd prefer to close the PR vs continuing a review cycle.

Stypox

@jmandel I'm sorry if my comment came off as rough, I didn't mean to demonize your work. I just wanted to make sure I wasn't just reviewing LLM code. I agree that "coming up with a design, implementing it, and testing all still require significant investment", and it's fine to use LLMs to quickly get up-to-speed with a codebase. But it's not good practice to open a PR just from the output of vibe coding, but it does not seem like this is what happened here, so no worries. I'm happy to continue with the reviews :-)

Could you add a few comments in the code explaining how the data flows from the various places? E.g. in DownloadStatusRepository, explain why the binding setup is needed, why its implementation is correct, and which parts are important to get right.

Stypox · 2025-09-30T16:18:13Z

app/src/main/res/values/strings.xml

+    <string name="download_open_failed">Unable to open downloaded file</string>
+    <string name="download_folder_open_failed">Unable to open folder</string>
+    <string name="download_delete_failed">Unable to delete downloaded file</string>
+    <string name="download_deleted">Deleted downloaded file</string>


I'm pretty sure most of these strings already exist, they are being used in the DownloadActivity, which I suggest you take a look at since most of the features you are implementing are already implemented there.

Removed the bespoke strings from strings.xml and switched the UI to the existing downloader vocabulary (missing_file, delete_file, delete_entry, etc.), so the detail sheet stays consistent with DownloadActivity.

Stypox · 2025-09-30T16:22:28Z

app/src/main/java/org/schabi/newpipe/download/DownloadDialog.java

    }
+
+    @Nullable
+    private String buildQualityLabel(@NonNull final Stream stream) {


Instead of duplicating code, could you create a common function and also use it in https://github.com/TeamNewPipe/NewPipe/blob/dev/app/src/main/java/org/schabi/newpipe/util/StreamItemAdapter.java#L113 ?

Created StreamLabelUtils.getQualityLabel(...) and replaced both the dialog helper and the adapter text formatting with calls to that function.

Stypox · 2025-09-30T16:26:10Z

app/src/main/java/org/schabi/newpipe/download/DownloadStatusRepository.kt

+    data object None : DownloadStatus
+    data class InProgress(val running: Boolean) : DownloadStatus


Why don't None and InProgress also have CompletedDownload info? Isn't that info also available there?

Introduced a unified DownloadEntry data class that carries the same metadata for pending, running, and finished missions, so each chip can surface identical details regardless of state.

Stypox · 2025-09-30T16:30:02Z

app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDownloadStatusViewModel.kt

+}
+
+data class DownloadUiState(
+    val chipState: DownloadChipState = DownloadChipState.Hidden,


Can't there be multiple downloads at once for the same video? Why show only one of them?

The repository/view-model now emit a list of DownloadEntrys keyed by their unique handle. The Compose surface renders all of them as chips and keeps the sheet focused on whichever entry was tapped.

Stypox · 2025-09-30T16:32:34Z

app/src/main/java/org/schabi/newpipe/download/ui/DownloadStatusUi.kt

+        DownloadChipState.Downloading -> AssistChip(
+            onClick = onChipClick,
+            label = { Text(text = stringResource(id = R.string.download_status_downloading)) }
+        )


I would show the same CompletedDownload information for pending downloads too. Just maybe show a different background, like it's currently done in DownloadActivity (with the bar-stripes background), so users can distinguish pending downloads from finished downloads.

Pending chips now show the same name/quality details and apply the marquee stripe styling sourced from Utility’s color logic, matching the visual language used in DownloadActivity.

app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDownloadStatusViewModel.kt

jmandel · 2025-10-04T01:31:42Z

No worries -- just wanted to make sure I'm contributing in a productive way. Really appreciate the review + comments here.

Could you add a few comments in the code explaining how the data flows from the various places? E.g. in DownloadStatusRepository, explain why the binding setup is needed, why its implementation is correct, and which parts are important to get right.

Added KDoc over observe()/withBinder() that walks through the lifetime of the service bind, the handler callback, and the teardown. The repository now always uses that helper, so there’s a single, documented spot managing the binder lifecycle.

Addressed other comments above inline.

Track downloaded streams and surface status on video detail

b5cb367

github-actions bot added the size/giant PRs with more than 750 changed lines label Sep 17, 2025

Stypox added feature request Issue is related to a feature in the app downloader Issue is related to the downloader labels Sep 17, 2025

jmandel marked this pull request as draft September 17, 2025 12:25

jmandel force-pushed the refactor branch 3 times, most recently from 206170f to 159e7bb Compare September 17, 2025 15:09

Refactor download status persistence and UI

13b10b6

Restore finshed mission conditoina

jmandel force-pushed the refactor branch from bdfa571 to 13b10b6 Compare September 17, 2025 16:41

Stypox requested changes Sep 30, 2025

View reviewed changes

jmandel marked this pull request as ready for review October 4, 2025 01:32

jmandel force-pushed the refactor branch 2 times, most recently from ae87a09 to a00e6e8 Compare October 4, 2025 01:35

Address PR Feedback

65ce28c

jmandel force-pushed the refactor branch from a00e6e8 to 65ce28c Compare October 4, 2025 01:42

jmandel requested a review from Stypox November 19, 2025 03:03

		data object None : DownloadStatus
		data class InProgress(val running: Boolean) : DownloadStatus

Uh oh!

Conversation

jmandel commented Sep 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What is it?

Description of the changes in your PR

Before/After Screenshots/Screen Record

Fixes the following issue(s)

Relies on the following changes

Uh oh!

Stypox commented Sep 17, 2025

Uh oh!

jmandel commented Sep 17, 2025

Uh oh!

Stypox commented Sep 17, 2025

Uh oh!

jmandel commented Sep 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Stypox left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jmandel commented Oct 4, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jmandel commented Sep 17, 2025 •

edited

Loading

jmandel commented Sep 17, 2025 •

edited

Loading